Skip to content

Unified KeyType API for multi-chain keyring with ICA HostKeyName support#12

Merged
akobrin1 merged 3 commits intomainfrom
unified-keyring
Feb 16, 2026
Merged

Unified KeyType API for multi-chain keyring with ICA HostKeyName support#12
akobrin1 merged 3 commits intomainfrom
unified-keyring

Conversation

@akobrin1
Copy link
Contributor

  • Added KeyType enum (KeyTypeCosmos, KeyTypeEVM) that encapsulates key algorithm, BIP44 HD path, and signing algo selection in one place
  • Unified keyring functions into NewKeyring, LoadKeyring, and ImportKey — a single keyring now supports both secp256k1 and eth_secp256k1 key types, with the algorithm chosen at import time
  • Removed redundant wrappers (NewKeyringEVM, LoadKeyringFromMnemonic, LoadKeyringFromMnemonicEVM, ImportKeyFromMnemonic, ImportKeyFromMnemonicEVM, NewMultiChainKeyring)
  • Added HostKeyName to ICA Config so controller and host chains can use independent key types
  • Extracted ICA types, constants, and errors into ica/types.go with doc comments
  • Updated documentation (README, API.md, DEVELOPER_GUIDE.md) with KeyType usage, ICA controller overview, strengths/limitations, and comparison with interchaintest
  • Added comprehensive unit tests (25 total) covering both key types, error paths, cross-HRP address derivation, and sign/verify round trips

@roomote
Copy link

roomote bot commented Feb 14, 2026

Rooviewer Clock   See task

Both previously flagged issues have been addressed. Latest commit (136bbde) is a doc comment fix only -- no new issues found.

  • LoadKeyring leaks a temp directory on every call (pkg/crypto/keyring.go) -- fixed with deferred cleanup on error paths and documented behavior on success path
  • ImportKey silently ignores keyType mismatch when a key with the same name already exists (pkg/crypto/keyring.go) -- fixed with algorithm verification and mismatch error
Previous reviews

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

roomote[bot]
roomote bot previously approved these changes Feb 14, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a unified multi-chain keyring API that simplifies key management across Cosmos and EVM-compatible chains. The main improvement is consolidating multiple chain-specific functions into a single keyring that supports both secp256k1 (Cosmos) and eth_secp256k1 (EVM) key types, with the algorithm selected at import time via a KeyType parameter.

Changes:

  • Introduced KeyType enum (KeyTypeCosmos, KeyTypeEVM) encapsulating algorithm, BIP44 HD path, and signing algo in one abstraction
  • Unified keyring functions: NewKeyring, LoadKeyring, and ImportKey now support both key types through explicit KeyType parameters
  • Removed redundant multi-chain wrappers (NewMultiChainKeyring, LoadKeyringFromMnemonicEVM, ImportKeyFromMnemonicEVM, etc.)
  • Extended ICA Config with HostKeyName field to support independent controller/host key types
  • Extracted ICA types, constants, and errors into ica/types.go for better organization
  • Updated all documentation and added 25 comprehensive unit tests covering both key types, error paths, and cross-chain scenarios

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/crypto/multichain_keyring.go Deleted - functionality consolidated into unified NewKeyring
pkg/crypto/keyring.go Added KeyType enum and unified NewKeyring/LoadKeyring/ImportKey functions supporting both Cosmos and EVM keys; removed redundant chain-specific wrappers
pkg/crypto/crypto_test.go Added 25 comprehensive tests for KeyType methods, unified keyring functions, multi-chain scenarios, error paths, and sign/verify round trips
ica/types.go New file extracting Config, Controller struct, constants, and error definitions from controller.go with improved documentation
ica/controller.go Integrated HostKeyName support, using it for host chain operations while KeyName signs controller chain transactions
examples/ica-request-verify/main.go Migrated from NewMultiChainKeyring to NewKeyring with KeyringParams
examples/ica-request-tx/main.go Migrated from NewMultiChainKeyring to NewKeyring with KeyringParams
docs/DEVELOPER_GUIDE.md Added comprehensive Crypto Helpers section and ICA Controller Overview with strengths/limitations and comparison with interchaintest
docs/API.md Added pkg/crypto and ica package API documentation with KeyType details and HostKeyName usage
README.md Updated crypto helpers section with KeyType enum, unified functions, and multi-chain key usage examples

@akobrin1 akobrin1 merged commit 3e807a1 into main Feb 16, 2026
2 checks passed
@akobrin1 akobrin1 deleted the unified-keyring branch February 16, 2026 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments